-
Notifications
You must be signed in to change notification settings - Fork 251
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Prevent error when running inline script plugin on empty stacktraces #563
Conversation
@@ -74,7 +74,7 @@ module.exports = { | |||
} | |||
|
|||
// only attempt to grab some surrounding code if we have a line number | |||
if (frame.lineNumber === undefined) return | |||
if (!frame || frame.lineNumber === undefined) return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, I did if (frame === undefined || frame.lineNumber === undefined) return
for consistency but in case you are checking for !frame
, might change the second condition to do the same? so...:
if (!frame || frame.lineNumber === undefined) return | |
if (!frame || !frame.lineNumber) return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left it as === undefined
because 0
is a valid number which evaluates as falsey. However, 0
isn't really a valid value for line number so arguably this is unnecessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SFTM
@bengourley, could you please release that fix within the next version? Would like to start using it. Thanks 😉 |
Sure, we've pencilled in a release at some point this week. Note that this isn't causing any issues or lost reports – the |
@bengourley After updating to 6.3.1 keep on getting this error And this is coming from |
I guess bugsnag/browser needs another release as well |
@bengourley perfect, thanks! |
Hi @bengourley! Was wondering why Thanks for the fix btw 🎉 |
It should be available on npm: https://www.npmjs.com/package/@bugsnag/js/v/6.3.2 |
Aw sorry my bad was looking at react plugin 😃 |
In the case of an error without a stacktrace, the
beforeSend
implemented by@bugsnag/plugin-inline-script-content
would throw an error (see #559).Since the logic that runs the list of
beforeSend
callbacks tolerates errors, the effect of this was pretty minimal – just some undesirable error logs output.The fix supplied by @andvla (thanks 🙏) was tweaked so that we won't attempt to look for
.lineNumber
on any falsey value forstacktrace[0]
.